feat: extend the tool to call the duplicate detection function to support pattern duplicate detection #5503

aheizi · 2025-07-09T06:28:08Z

Related GitHub Issue

Description

The Tool Repetition Detector class has been enhanced and the pattern repetition detection function has been added, which can detect repetitive patterns like "ABCABC"

Test Procedure

unit test

Test the simple alternating mode (ABABAB)
Test the complex three-element pattern (ABCABCABC)
Test the pattern detection of different parameter values
Test the repetition limit of the custom mode

Pre-Submission Checklist

Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
Scope: My changes are focused on the linked issue (one major feature/fix per PR).
Self-Review: I have performed a thorough self-review of my code.
Testing: New and/or updated tests have been added to cover my changes (if applicable).
Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes

Get in Touch

aheizi

Important

Enhance ToolRepetitionDetector to detect pattern repetitions and add corresponding tests.

Behavior:
- Extend ToolRepetitionDetector to detect pattern repetitions like "ABCABC" in ToolRepetitionDetector.ts.
- Introduce historyMaxLength and patternRepetitionLimit parameters in constructor.
- Add updateHistory(), detectPatternRepetition(), and hasRepeatingPattern() methods for pattern detection.
- Reset state after detection using resetState().
Tests:
- Add tests in ToolRepetitionDetector.spec.ts for pattern detection (e.g., "ABABAB", "ABCABCABC").
- Test different parameter values and custom repetition limits.
- Ensure state reset after pattern detection.

^{This description was created by}^{for bd331ed788f133f80b488cce4f914a2f47b8c99a. You can customize this summary. It will automatically update as commits are pushed.}

daniel-lxs

Hey @aheizi Thank you for your contribution, I think this is an interesting approach to handling more complex forms of looping.

I left some suggestions that are worth taking a look at.

Let me know if you have any questions!

src/core/tools/ToolRepetitionDetector.ts

src/core/tools/__tests__/ToolRepetitionDetector.spec.ts

aheizi · 2025-07-15T13:39:06Z

Hey @aheizi Thank you for your contribution, I think this is an interesting approach to handling more complex forms of looping.

I left some suggestions that are worth taking a look at.

Let me know if you have any questions!

@daniel-lxs Thank you for the review. I have revised and submitted it.

daniel-lxs

Thank you @aheizi, I left a couple of suggestions to help improve your implementation.

Let me know what you think!

src/core/tools/ToolRepetitionDetector.ts

daniel-lxs · 2025-07-21T15:48:17Z

src/core/tools/__tests__/ToolRepetitionDetector.spec.ts

I noticed that after pattern detection triggers and resets, the next tool call still fails (line 468-469). Is this intentional?

If the goal of resetState() is to allow recovery after hitting a limit, shouldn't the next call be allowed? The current behavior could trap users in a situation where they can't recover from pattern detection.

Could you clarify the intended behavior here? If this is intentional, it might be worth adding a comment explaining why.

Added a comment explaining why.

daniel-lxs · 2025-07-21T15:48:58Z

src/core/tools/ToolRepetitionDetector.ts

Consider adding parameter validation in the constructor. This would prevent issue if someone passes negative values or extremely large values for historyLength or patternLimit.

For example:

constructor(consecutiveLimit: number = 3, historyLength: number = 20, patternLimit: number = 2) { this.consecutiveIdenticalToolCallLimit = Math.max(0, consecutiveLimit) this.historyMaxLength = Math.max(1, Math.min(historyLength, 1000)) // reasonable upper bound this.patternRepetitionLimit = Math.max(0, patternLimit) }

This would prevent potential issues with invalid parameters.

daniel-lxs · 2025-07-21T15:49:34Z

src/core/tools/ToolRepetitionDetector.ts

When consecutiveIdenticalToolCallLimit is 0 (unlimited), the code skips all repetition checks including pattern detection. Is this intentional?

It seems like pattern repetition (ABABAB) could still be relevant even when consecutive repetitions (AAA) are unlimited. Should these two types of detection be independent?

If the current behavior is intentional, could you add a comment clarifying that setting consecutive limit to 0 disables ALL repetition detection, not just consecutive?

Added a comment clarifying that setting consecutive limit to 0 disables ALL repetition detection.

daniel-lxs · 2025-07-21T15:49:51Z

src/core/tools/ToolRepetitionDetector.ts

The magic number 5 for history retention should be extracted to a constant or made configurable for better maintainability.

Consider:

private static readonly HISTORY_RETENTION_ON_RESET = 5; private resetState(): void { this.consecutiveIdenticalToolCallCount = 0 this.previousToolCallJson = null // Keep last N entries for context this.toolCallHistory = this.toolCallHistory.slice(-ToolRepetitionDetector.HISTORY_RETENTION_ON_RESET) }

This makes it easier to adjust the value if needed and clarifies the intent.

src/core/tools/ToolRepetitionDetector.ts

…port pattern duplicate detection

aheizi · 2025-07-26T02:35:54Z

@daniel-lxs Thank you for the review. I corrected some issues and adjusted some of the annotations to make them more accurate.

aheizi · 2025-07-26T02:42:22Z

@daniel-lxs There’s another issue: currently, consecutiveLimit can be set to 1, which means the tool gets blocked on its first invocation. However, this doesn’t align well with the message shown to users:

Number of consecutive errors or repeated actions allowed before the “WeCoder encountered a problem” dialog appears

Since there’s no repetition at that point, the behavior feels inconsistent.

There are three possible solutions:

Clearly define that consecutiveLimit = 1 means “allow 1 repeated call, block on the 2nd identical call”; consecutiveLimit = 2 means “allow 2 repeated calls, block on the 3rd”, and so on.
When users set the value to 1, clearly inform them: “First-time invocation restriction is enabled. Every call will be restricted.”
Disallow setting consecutiveLimit to 1.

I’m leaning towards option 2. If you’re also okay with it, I’ll add a user-facing message accordingly. Looking forward to your feedback.

daniel-lxs · 2025-07-27T17:43:53Z

Hey @aheizi Good catch. I agree a message is the right approach here. That said, I think it should be clearer. “First-time invocation restriction is enabled” doesn’t make it obvious that setting consecutiveLimit to 1 means the tool will stop immediately after the first error, even if it’s not repeated. We should make that behavior explicit so users know it’s not about repetition at that point, but a strict 1-error threshold.

aheizi · 2025-07-28T09:52:19Z

Hey @aheizi Good catch. I agree a message is the right approach here. That said, I think it should be clearer. “First-time invocation restriction is enabled” doesn’t make it obvious that setting consecutiveLimit to 1 means the tool will stop immediately after the first error, even if it’s not repeated. We should make that behavior explicit so users know it’s not about repetition at that point, but a strict 1-error threshold.

Hey @daniel-lxs
The current implementation, setting consecutiveLimit to 1, means that each tool call will be judged as a duplicate and fail.

This is the performance of the todo list for the first time after it is set to 1：

aheizi · 2025-07-28T09:58:04Z

Or like this:
⚠️ Any tool call will be immediately interrupted. It is recommended to use it only during debugging.

daniel-lxs · 2025-07-28T15:12:43Z

Hey @aheizi, thank you for your work on this. After careful consideration, I believe this change could affect valid repetitive workflows that shouldn't be flagged as looping, such as editing files and running tests.

This doesn't mean your implementation isn't solid or that we don't appreciate it. I'm going to close the PR for now, but I'm open to continuing the discussion.

aheizi · 2025-07-28T15:40:07Z

@daniel-lxs The problem of repeatedly calling tools that I am currently encountering is quite frequent. Although I am also using the cloude4 model, it is still very difficult to avoid the situation of pattern repetition. The situation you mentioned where there might be misjudgments when editing files and running tests, I think, also exists in the previous consecutive repetitive scenarios. Anyway, I still very much hope that this problem can be solved from an engineering perspective.

aheizi requested review from cte, jr and mrubens as code owners July 9, 2025 06:28

github-project-automation bot added this to Roo Code Roadmap and Roo Code Roadmap Jul 9, 2025

github-project-automation bot moved this to New in Roo Code Roadmap Jul 9, 2025

github-project-automation bot moved this to Triage in Roo Code Roadmap Jul 9, 2025

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jul 9, 2025

hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 9, 2025

daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jul 9, 2025

hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 9, 2025

daniel-lxs reviewed Jul 14, 2025

View reviewed changes

daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 14, 2025

hannesrudolph added PR - Changes Requested and removed PR - Needs Preliminary Review labels Jul 15, 2025

aheizi force-pushed the optimize-tools-repetition-detector branch 2 times, most recently from 95de640 to 646bdd2 Compare July 16, 2025 14:14

daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 16, 2025

hannesrudolph added PR - Needs Preliminary Review and removed PR - Changes Requested labels Jul 16, 2025

daniel-lxs reviewed Jul 21, 2025

View reviewed changes

daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jul 21, 2025

daniel-lxs moved this from PR [Needs Review] to PR [Changes Requested] in Roo Code Roadmap Jul 21, 2025

hannesrudolph added PR - Changes Requested and removed PR - Needs Preliminary Review labels Jul 21, 2025

aheizi commented Jul 23, 2025

View reviewed changes

src/core/tools/ToolRepetitionDetector.ts Outdated Show resolved Hide resolved

aheizi force-pushed the optimize-tools-repetition-detector branch from 646bdd2 to 372bf8c Compare July 23, 2025 15:51

feat: extend the tool to call the duplicate detection function to sup…

bf71632

…port pattern duplicate detection

aheizi force-pushed the optimize-tools-repetition-detector branch from 372bf8c to bf71632 Compare July 26, 2025 02:38

daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 28, 2025

hannesrudolph added PR - Needs Preliminary Review and removed PR - Changes Requested labels Jul 28, 2025

daniel-lxs closed this Jul 28, 2025

github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 28, 2025

github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jul 28, 2025

daniel-lxs mentioned this pull request Jul 28, 2025

Add the detection of non-continuous tool repeated calls #5496

Closed

4 tasks

feat: extend the tool to call the duplicate detection function to support pattern duplicate detection #5503

feat: extend the tool to call the duplicate detection function to support pattern duplicate detection #5503

Uh oh!

Conversation

aheizi commented Jul 9, 2025 • edited by daniel-lxs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related GitHub Issue

Description

Test Procedure

Pre-Submission Checklist

Screenshots / Videos

Documentation Updates

Additional Notes

Get in Touch

Uh oh!

daniel-lxs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aheizi commented Jul 15, 2025

Uh oh!

daniel-lxs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aheizi commented Jul 26, 2025

Uh oh!

aheizi commented Jul 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daniel-lxs commented Jul 27, 2025

Uh oh!

aheizi commented Jul 28, 2025

Uh oh!

aheizi commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daniel-lxs commented Jul 28, 2025

Uh oh!

aheizi commented Jul 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aheizi commented Jul 9, 2025 •

edited by daniel-lxs

Loading

aheizi commented Jul 26, 2025 •

edited

Loading

aheizi commented Jul 28, 2025 •

edited

Loading